Improve tracing options in JobManager and DeadlockDetector#2587
Improve tracing options in JobManager and DeadlockDetector#2587fedejeanne merged 3 commits intoeclipse-platform:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the observability of job scheduling/locking behavior in org.eclipse.core.jobs by expanding trace options and routing deadlock/lock diagnostics through the standard tracing mechanism instead of System.out.
Changes:
- Adds new trace options (
jobs/yielding,jobs/yielding/detailed,jobs/blockedunblocked) to the bundle’s.optionsso they can be toggled via Eclipse tracing preferences. - Replaces
System.out.printlncalls inDeadlockDetectorwithJobManager.debug(...)for consistency with existing tracing. - Updates
JobManager.reportBlocked(...)to include the blocking job’s name in the blocked message.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
runtime/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java |
Changes blocked-status reporting to name the blocking job (but currently also changes the status type/contents). |
runtime/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/DeadlockDetector.java |
Routes lock/deadlock debug output through the bundle debug trace mechanism. |
runtime/bundles/org.eclipse.core.jobs/.options |
Exposes additional tracing toggles via the Eclipse “General > Tracing” UI. |
Comments suppressed due to low confidence (1)
runtime/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java:1403
reportBlockednow creates aJobStatus(exposing the blockingJobviaIJobStatus#getJob()) for all blocking jobs. Previously, system jobs andThreadJobblockers were intentionally mapped to a plainStatusto avoid leaking internal/system jobs to progress monitors and to avoid confusing names like "Implicit Job" in the user-facing message.
Consider restoring the special-casing at least for ThreadJob (and possibly isSystem()), while still including the blocker’s name in the message if desired. A safe approach is to always build the message, but only wrap it in JobStatus for non-system, non-ThreadJob jobs; otherwise use a plain Status so clients can’t obtain the internal job instance from the blocked reason.
InternalJob blockingJob = blockingJobs.stream().sorted(Comparator.comparing(InternalJob::isSystem)).findFirst()
.orElse(null);
if (blockingJob == null) {
reason = new Status(IStatus.INFO, JobManager.PI_JOBS, 1, JobMessages.jobs_blocked0, null);
} else {
String msg = NLS.bind(JobMessages.jobs_blocked1, blockingJob.getName());
reason = new JobStatus(IStatus.INFO, (Job) blockingJob, msg);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This pull request changes some projects for the first time in this development cycle. Warning 🚧 This PR cannot be modified by maintainers because edits are disabled or it is created from an organization repository. To obtain the required changes apply the git patch manually as an additional commit. Git patchFurther information are available in Common Build Issues - Missing version increments. |
Add the following to the .options file so that one can choose to trace these events via the preferences (General > Tracing): - jobs/yielding - jobs/yielding/detailed - jobs/blockedunblocked Do not log to sysout for the preference "jobs/locks", use the same method as other tracers. Show the name of system jobs that block other jobs in JobManager.reportBlocked(...)
2efc798 to
df90062
Compare
|
If there are no objections, I plan to merge this week |
Do not set them to false if they are not explicitly set in the configuration, leave them as they are. This avoids the duplicated logic of declaring them as "false" when they are declared and also when they are loaded.
Add the following to the .options file so that one can choose to trace these events via the preferences (
General > Tracing):Do not log to sysout for the preference "jobs/locks", use the same method as other tracers.
Show the name of system jobs that block other jobs in
JobManager.reportBlocked(...)